From 800d9eb86a6af046bd5def1e1c502b0ad58ca071 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 3 Nov 2014 16:23:42 -0800 Subject: [PATCH] Fix the build command passing -L/-l without links --- src/cargo/ops/cargo_rustc/context.rs | 9 ++-- src/cargo/ops/cargo_rustc/custom_build.rs | 63 ++++++++++++++++------- src/cargo/ops/cargo_rustc/mod.rs | 34 ++++++------ tests/test_cargo_compile_custom_build.rs | 5 +- 4 files changed, 67 insertions(+), 44 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index f6ca433fa..b2bd59eb2 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -1,7 +1,7 @@ use std::collections::HashSet; use std::collections::hash_map::{HashMap, Occupied, Vacant}; use std::str; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use core::{SourceMap, Package, PackageId, PackageSet, Resolve, Target}; use util::{mod, CargoResult, ChainError, internal, Config, profile}; @@ -9,6 +9,7 @@ use util::human; use super::{Kind, KindHost, KindTarget, Compilation, BuildOutput}; use super::layout::{Layout, LayoutProxy}; +use super::custom_build::BuildState; #[deriving(Show)] pub enum PlatformRequirement { @@ -22,7 +23,7 @@ pub struct Context<'a, 'b: 'a> { pub resolve: &'a Resolve, pub sources: &'a SourceMap<'b>, pub compilation: Compilation, - pub native_libs: Arc>>, + pub build_state: Arc, env: &'a str, host: Layout, @@ -40,7 +41,7 @@ impl<'a, 'b: 'a> Context<'a, 'b> { deps: &'a PackageSet, config: &'b Config<'b>, host: Layout, target: Option, root_pkg: &Package, - native_libs: HashMap) + build_state: HashMap) -> CargoResult> { let (target_dylib, target_exe) = try!(Context::filename_parts(config.target())); @@ -66,7 +67,7 @@ impl<'a, 'b: 'a> Context<'a, 'b> { host_dylib: host_dylib, requirements: HashMap::new(), compilation: Compilation::new(root_pkg), - native_libs: Arc::new(Mutex::new(native_libs)), + build_state: Arc::new(BuildState::new(build_state, deps)), }) } diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 727aebaec..c11c64f6b 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -1,9 +1,11 @@ +use std::collections::HashMap; use std::fmt; use std::io::fs::PathExtensions; use std::io::{fs, USER_RWX, File}; use std::str; +use std::sync::Mutex; -use core::{Package, Target}; +use core::{Package, Target, PackageId, PackageSet}; use util::{CargoResult, CargoError, human}; use util::{internal, ChainError, Require}; @@ -22,6 +24,10 @@ pub struct BuildOutput { pub metadata: Vec<(String, String)>, } +pub struct BuildState { + pub outputs: Mutex>, +} + /// Prepares a `Work` that executes the target as a custom build script. pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) -> CargoResult<(Work, Work, Freshness)> { @@ -40,7 +46,7 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) // Start preparing the process to execute, starting out with some // environment variables. let profile = target.get_profile(); - let mut p = super::process(to_exec, pkg, target, cx) + let mut p = try!(super::process(to_exec, pkg, target, cx)) .env("OUT_DIR", Some(&build_output)) .env("CARGO_MANIFEST_DIR", Some(pkg.get_manifest_path() .dir_path() @@ -74,13 +80,15 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) !t.get_profile().is_custom_build() }).unwrap(); cx.dep_targets(pkg, non_build_target).iter().filter_map(|&(pkg, _)| { - pkg.get_manifest().get_links() - }).map(|s| s.to_string()).collect::>() + pkg.get_manifest().get_links().map(|links| { + (links.to_string(), pkg.get_package_id().clone()) + }) + }).collect::>() }; - let lib_name = pkg.get_manifest().get_links().map(|s| s.to_string()); let pkg_name = pkg.to_string(); - let native_libs = cx.native_libs.clone(); - let all = (lib_name.clone(), pkg_name.clone(), native_libs.clone(), + let build_state = cx.build_state.clone(); + let id = pkg.get_package_id().clone(); + let all = (id.clone(), pkg_name.clone(), build_state.clone(), script_output.clone(), old_build_output.clone(), build_output.clone()); @@ -109,11 +117,11 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) // along to this custom build command. let mut p = p; { - let native_libs = native_libs.lock(); - for dep in lib_deps.iter() { - for &(ref key, ref value) in (*native_libs)[*dep].metadata.iter() { + let build_state = build_state.outputs.lock(); + for &(ref name, ref id) in lib_deps.iter() { + for &(ref key, ref value) in (*build_state)[*id].metadata.iter() { p = p.env(format!("DEP_{}_{}", - super::envify(dep.as_slice()), + super::envify(name.as_slice()), super::envify(key.as_slice())).as_slice(), Some(value.as_slice())); } @@ -139,10 +147,7 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) human("build script output was not valid utf-8") })); let build_output = try!(BuildOutput::parse(output, pkg_name.as_slice())); - match lib_name { - Some(name) => assert!(native_libs.lock().insert(name, build_output)), - None => {} - } + build_state.outputs.lock().insert(id, build_output); try!(File::create(&script_output.join("output")) .write_str(output).map_err(|e| { @@ -166,7 +171,7 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) try!(fingerprint::prepare_build_cmd(cx, pkg, Some(target))); let dirty = proc(tx: Sender) { try!(work(tx.clone())); dirty(tx) }; let fresh = proc(tx) { - let (lib_name, pkg_name, native_libs, script_output, + let (id, pkg_name, build_state, script_output, old_build_output, build_output) = all; let new_loc = script_output.join("output"); try!(fs::rename(&old_script_output.join("output"), &new_loc)); @@ -177,10 +182,7 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) let contents = try!(f.read_to_string()); let output = try!(BuildOutput::parse(contents.as_slice(), pkg_name.as_slice())); - match lib_name { - Some(name) => assert!(native_libs.lock().insert(name, output)), - None => {} - } + build_state.outputs.lock().insert(id, output); fresh(tx) }; @@ -188,6 +190,27 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) Ok((dirty, fresh, freshness)) } +impl BuildState { + pub fn new(overrides: HashMap, + packages: &PackageSet) -> BuildState { + let mut sources = HashMap::new(); + for package in packages.iter() { + match package.get_manifest().get_links() { + Some(links) => { + sources.insert(links.to_string(), + package.get_package_id().clone()); + } + None => {} + } + } + let mut outputs = HashMap::new(); + for (name, output) in overrides.into_iter() { + outputs.insert(sources[name].clone(), output); + } + BuildState { outputs: Mutex::new(outputs) } + } +} + impl BuildOutput { // Parses the output of a script. // The `pkg_name` is used for error messages. diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index a3a9c83d5..be5dcf86e 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -2,7 +2,6 @@ use std::collections::{HashSet, HashMap}; use std::dynamic_lib::DynamicLibrary; use std::io::{fs, USER_RWX}; use std::io::fs::PathExtensions; -use std::os; use core::{SourceMap, Package, PackageId, PackageSet, Target, Resolve}; use util::{mod, CargoResult, ProcessBuilder, CargoError, human, caused_human}; @@ -173,13 +172,9 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, if target.get_profile().is_custom_build() { // Custom build commands that are for libs that are overridden are // skipped entirely - match pkg.get_manifest().get_links() { - Some(lib) => { - if cx.native_libs.lock().contains_key_equiv(lib) { - continue - } - } - None => {} + if pkg.get_manifest().get_links().is_some() && + cx.build_state.outputs.lock().contains_key(pkg.get_package_id()) { + continue } let (dirty, fresh, freshness) = try!(custom_build::prepare(pkg, target, cx)); @@ -343,17 +338,20 @@ fn rustc(package: &Package, target: &Target, let rustc = if show_warnings {rustc} else {rustc.arg("-Awarnings")}; // Prepare the native lib state (extra -L and -l flags) - let native_libs = cx.native_libs.clone(); + let build_state = cx.build_state.clone(); let mut native_lib_deps = Vec::new(); + let current_id = package.get_package_id().clone(); + let has_custom_build = package.get_targets().iter().any(|t| { + t.get_profile().is_custom_build() + }); // FIXME: traverse build dependencies and add -L and -l for an // transitive build deps. if !target.get_profile().is_custom_build() { each_dep(package, cx, |dep| { - let primary = package.get_package_id() == dep.get_package_id(); - match dep.get_manifest().get_links() { - Some(name) => native_lib_deps.push((name.to_string(), primary)), - None => {} + if dep.get_manifest().get_links().is_some() || + (*dep.get_package_id() == current_id && has_custom_build) { + native_lib_deps.push(dep.get_package_id().clone()); } }); } @@ -364,13 +362,13 @@ fn rustc(package: &Package, target: &Target, // Only at runtime have we discovered what the extra -L and -l // arguments are for native libraries, so we process those here. { - let native_libs = native_libs.lock(); - for &(ref lib, primary) in native_lib_deps.iter() { - let output = &(*native_libs)[*lib]; + let build_state = build_state.outputs.lock(); + for id in native_lib_deps.iter() { + let output = &(*build_state)[*id]; for path in output.library_paths.iter() { rustc = rustc.arg("-L").arg(path); } - if primary { + if *id == current_id { for name in output.library_links.iter() { rustc = rustc.arg("-l").arg(name.as_slice()); } @@ -490,7 +488,7 @@ fn build_base_args(cx: &Context, .rpath(root_profile.get_rpath()) } - if profile.is_plugin() { + if profile.is_for_host() { cmd = cmd.arg("-C").arg("prefer-dynamic"); } diff --git a/tests/test_cargo_compile_custom_build.rs b/tests/test_cargo_compile_custom_build.rs index 52bd0b2e0..e83bbdadd 100644 --- a/tests/test_cargo_compile_custom_build.rs +++ b/tests/test_cargo_compile_custom_build.rs @@ -668,7 +668,8 @@ test!(build_cmd_with_a_build_cmd { --out-dir [..]target[..]deps --dep-info [..]fingerprint[..]dep-lib-a \ -L [..]target[..]deps -L [..]target[..]deps` {compiling} foo v0.5.0 (file://[..]) -{running} `rustc build.rs --crate-name build-script-build --crate-type bin -g \ +{running} `rustc build.rs --crate-name build-script-build --crate-type bin \ + -C prefer-dynamic -g \ --out-dir [..]build[..]foo-[..] --dep-info [..]fingerprint[..]dep-[..] \ -L [..]target -L [..]target[..]deps \ --extern a=[..]liba-[..].rlib` @@ -744,7 +745,7 @@ test!(output_separate_lines { } "#); assert_that(p.cargo_process("build").arg("-v"), - execs().with_status(0) + execs().with_status(101) .with_stdout(format!("\ {compiling} foo v0.5.0 (file://[..]) {running} `rustc build.rs [..]` -- 2.30.2